Conversation
0b79786 to
c396b0a
Compare
4b45d46 to
8f5dcd9
Compare
f3080f1 to
8a987ad
Compare
aec36e8 to
0e0e9a4
Compare
0e0e9a4 to
d761679
Compare
alexey-pelykh
left a comment
There was a problem hiding this comment.
Thanks for the clean implementation and for incorporating all the review feedback.
The module is well-structured and the rounding logic is solid. CI passes (tests + pre-commit), and ivantodorovich already approved. Just a couple of minor observations on the current state:
Dead code in tests
_get_invalid_qty_warning_msg (and the formatLang import) in the test file appear to be leftover from a previous iteration that included validation/warning logic. Since the constraint was removed in favor of silent onchange rounding, this helper is never called. Worth cleaning up to avoid confusion.
Template compute/inverse test coverage
The product_template.py compute/inverse logic for syncing sale_multiple_uom_id between template and single-variant product is not exercised by the current tests. A small test that sets the field on a template and verifies it propagates to the variant (and vice versa) would strengthen coverage and likely address the codecov gaps.
Neither of these is blocking -- the core functionality is correct and well-tested.
|
Hello @alexey-pelykh
|
d761679 to
17419af
Compare
| help="When set, sale order quantities are rounded up to an " | ||
| "multiple number of this unit.", |
There was a problem hiding this comment.
| help="When set, sale order quantities are rounded up to an " | |
| "multiple number of this unit.", | |
| help="When set, sale order quantities are rounded up to a " | |
| "multiple number of this unit.", |
| help="When set, sale order quantities are rounded up to an " | ||
| "multiple number of this unit.", |
There was a problem hiding this comment.
| help="When set, sale order quantities are rounded up to an " | |
| "multiple number of this unit.", | |
| help="When set, sale order quantities are rounded up to a " | |
| "multiple number of this unit.", |
| If the Sales Multiple UoM is not divisible by the order line UoM, the rounded | ||
| quantity may be fractional. It is the user's responsibility to | ||
| configure compatible units of measure. | ||
|
|
|
This PR has the |
| return qty_rounded | ||
|
|
||
| @api.onchange("product_id", "product_uom_qty", "product_uom_id") | ||
| def _onchange_product_uom_qty_round_multiple(self): |
There was a problem hiding this comment.
IMHO, as product_uom_qty is a computed field and onchanges should be avoided, this should be changed to compute_product_uom_qty()
There was a problem hiding this comment.
AFAIR we used onchange to not have this UX rounding play out for programatic assignation, and keep it only for forms.
In case of programatic assignation, no autofix is done and the constraint plays out -- which is better to detect possible errors
There was a problem hiding this comment.
Our goal is to propose the rounded value to a user. That's it.
AFAIR, the input value still can be used and there is no real impact anywhere except the forms.
Moreover, we dropped an idea to have any constraints here to allow rounding with decimals, because only the UoM's that share "Unit" as a common reference and have an integer as a sales multiple.
We probably might add a tiny improvement for the rounding, so the UoM's that share "Unit" as a common reference are rounded to the nearest multiple integer.
So, this example:
order line UoM: Pack of 6 (6 units)
sales multiple UoM: Pack of 100 (100 units)
ordering 13 packs of 6 units (78 units) is rounded to 16.67 packs (100.02 units);
Will have the rounded value 17.
17419af to
a7aecaf
Compare
This module adds a Sales Multiple unit of measure on products.
When a Sales Multiple is set, sales order line quantities are automatically
rounded UP to the nearest multiple of the selected unit of measure.
This is useful for products that must be sold in fixed pack sizes
(boxes, bundles, pallets, etc.).
The rounding is performed by converting the entered quantity to the Sales
Multiple UoM, rounding the number of packs UP to the nearest integer,
and converting the result back to the order line UoM.
Being said, if sales multiple UoM is divisible by the order line UoM, the result will be an integer.
Otherwise, we expect rounding issues.
For example (compatible UoMs: 100 is divisible by 5):
For example (fractional result: 100 is not divisible by 6):
If the Sales Multiple UoM is not divisible by the order line UoM, the rounded
quantity may be fractional. In such cases, the module displays a warning suggesting the nearest valid integer and
prevents saving invalid quantities. It is the user's responsibility to
configure compatible units of measure.